[PATCH] QML engine: Correctly compare composites when multiple engines are used

Since 6.11, we allow multiple types to be active for the same URL.
While we converted multiple places to handle this, we missed adjusting
the property validator. There, we just picked the last inserted property
cache, which can lead to validation failures.

Fix this by adding a fallback path which checks all potential matches
when the initial lookup would yield an error.

The same kind of check is also needed in qmlobject_can_qml_cast.

Amends 9cc23ca91c404f24fff3c36e8c9425ae674f341d.

Change compared to 6.12:
Dropped the isComposite check, which doesn't exist there. That causes
one potential extra unneeded lock acquisition, but only in a case
where'd we get an error anyway.

Fixes: QTBUG-149607
Change-Id: Ibd0df2325807e75b23056ebc0bacdfe877b002f3
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
(cherry picked from commit 94a76ed58629932f6d689c48ecf8d3132188923b)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 8c23f65ec4889f9ff7f609002a129698796de2b8)

Gbp-Pq: Name upstream_2efb7c6_correctly_compare_composites_with_multiple_engines.diff

diff --git a/src/qml/qml/qqmlglobal.cpp b/src/qml/qml/qqmlglobal.cpp
index 3e11b861291168949e5b847cf9905dd0c85aecd3..e4d94e4d8e89cf901776d3eb54fa3ca1c7b62514 100644 (file)
--- a/src/qml/qml/qqmlglobal.cpp
+++ b/src/qml/qml/qqmlglobal.cpp
@@ -1112,9 +1112,9 @@ bool qmlobject_can_qml_cast(QObject *object, const QQmlType &type)
 
     // A non-composite type will always have a metaobject.
     const QMetaObject *typeMetaObject = type.metaObject();
-    const QQmlPropertyCache::ConstPtr typePropertyCache = typeMetaObject
-            ? QQmlPropertyCache::ConstPtr()
-            : QQmlMetaType::findPropertyCacheInCompositeTypes(type.typeId());
+    QVarLengthArray<QQmlPropertyCache::ConstPtr, 4> allPropertyCacheCandidates =
+            typeMetaObject ? QVarLengthArray<QQmlPropertyCache::ConstPtr, 4>{}
+                           : QQmlMetaType::rawCompositePropertyCachesForType(type.typeId());
 
     if (const QQmlData *ddata = ddata_for_cast(object)) {
         for (const QQmlPropertyCache *propertyCache = ddata->propertyCache.data(); propertyCache;
@@ -1134,17 +1134,17 @@ bool qmlobject_can_qml_cast(QObject *object, const QQmlType &type)
                 // property caches to be unrelated but the types still convertible.
                 // Multiple property caches can hold the same metaobject, for example for
                 // versions of non-composite types.
-                if (propertyCache == typePropertyCache.data())
+                if (allPropertyCacheCandidates.contains(propertyCache))
                     return true;
             }
         }
     }
 
-    // If nothing else works, we have to create the metaobjects.
+    // If nothing else works, we have to create the metaobjects (if we can).
+    if (!typeMetaObject && !allPropertyCacheCandidates.isEmpty())
+        typeMetaObject = allPropertyCacheCandidates.first()->createMetaObject();
 
-    return object->metaObject()->inherits(typeMetaObject
-            ? typeMetaObject
-            : (typePropertyCache ? typePropertyCache->createMetaObject() : nullptr));
+    return object->metaObject()->inherits(typeMetaObject);
 }
 
 QT_END_NAMESPACE

diff --git a/src/qml/qml/qqmlmetatype.cpp b/src/qml/qml/qqmlmetatype.cpp
index 173c21d9c45405df5f6b625addecbfdcbf0af9da..1703c99dd129777b1c9498ee550e5a32898fbbf5 100644 (file)
--- a/src/qml/qml/qqmlmetatype.cpp
+++ b/src/qml/qml/qqmlmetatype.cpp
@@ -1487,6 +1487,20 @@ QQmlPropertyCache::ConstPtr QQmlMetaType::rawPropertyCacheForType(QMetaType meta
             : QQmlPropertyCache::ConstPtr();
 }
 
+/*!
+ * \internal
+ *
+ * Returns all candidate property caches for a composite
+ * metatype instead of only the last inserted one.
+ * compare rawPropertyCacheForType (which handles however also non-composites)
+ */
+QVarLengthArray<QQmlPropertyCache::ConstPtr, 4>
+QQmlMetaType::rawCompositePropertyCachesForType(QMetaType metaType)
+{
+    const QQmlMetaTypeDataPtr data;
+    return data->findPropertyCachesInCompositeTypes(metaType);
+}
+
 /*!
  * \internal
  *

diff --git a/src/qml/qml/qqmlmetatype_p.h b/src/qml/qml/qqmlmetatype_p.h
index 62b6cba778cd816201bbc7a8f3d7e891d6f09c33..040a42d1f7eb3b8bed59b132338ba2b614011f02 100644 (file)
--- a/src/qml/qml/qqmlmetatype_p.h
+++ b/src/qml/qml/qqmlmetatype_p.h
@@ -23,6 +23,7 @@
 #include <private/qtqmlglobal_p.h>
 
 #include <QtCore/qtyperevision.h>
+#include <QtCore/qvarlengtharray.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -171,6 +172,10 @@ public:
     static QQmlPropertyCache::ConstPtr rawPropertyCacheForType(
             QMetaType metaType, QTypeRevision version);
 
+    // All property caches for a composite metatype, which may map to more than one of them.
+    static QVarLengthArray<QQmlPropertyCache::ConstPtr, 4> rawCompositePropertyCachesForType(
+            QMetaType metaType);
+
     static bool canConvert(QObject *o, QMetaType metaType);
     static bool canConvert(const QQmlPropertyCache::ConstPtr &from, QMetaType metaType);
 

diff --git a/src/qml/qml/qqmlmetatypedata.cpp b/src/qml/qml/qqmlmetatypedata.cpp
index e72a3a97195e88bee3ee3aa4c6826523065562a4..41d7d8c38c63366bb9baef7dbf95af5b4ffa46f5 100644 (file)
--- a/src/qml/qml/qqmlmetatypedata.cpp
+++ b/src/qml/qml/qqmlmetatypedata.cpp
@@ -257,6 +257,18 @@ QQmlPropertyCache::ConstPtr QQmlMetaTypeData::findPropertyCacheInCompositeTypes(
             : propertyCacheForPotentialInlineComponentType(t, iter);
 }
 
+QVarLengthArray<QQmlPropertyCache::ConstPtr, 4>
+QQmlMetaTypeData::findPropertyCachesInCompositeTypes(QMetaType t) const
+{
+    QVarLengthArray<QQmlPropertyCache::ConstPtr, 4> result;
+    const auto [begin, end] = compositeTypes.equal_range(t.iface());
+    for (auto iter = begin; iter != end; ++iter) {
+        if (auto cache = propertyCacheForPotentialInlineComponentType(t, iter))
+            result.append(std::move(cache));
+    }
+    return result;
+}
+
 void QQmlMetaTypeData::clearCompositeTypes()
 {
     // Unregister all remaining composite types.

diff --git a/src/qml/qml/qqmlmetatypedata_p.h b/src/qml/qml/qqmlmetatypedata_p.h
index ce13c4027e7721b494aece41b0946d3716e901bd..95c60a1581a76c538017d8702d909475d4a32b57 100644 (file)
--- a/src/qml/qml/qqmlmetatypedata_p.h
+++ b/src/qml/qml/qqmlmetatypedata_p.h
@@ -23,6 +23,7 @@
 #include <private/qqmlvaluetype_p.h>
 
 #include <QtCore/qset.h>
+#include <QtCore/qvarlengtharray.h>
 #include <QtCore/qvector.h>
 
 QT_BEGIN_NAMESPACE
@@ -119,6 +120,10 @@ struct QQmlMetaTypeData
     QQmlPropertyCache::ConstPtr propertyCache(const QQmlType &type, QTypeRevision version);
     QQmlPropertyCache::ConstPtr findPropertyCacheInCompositeTypes(QMetaType t) const;
 
+    // Same, but returns all matches rather than only the last inserted one.
+    QVarLengthArray<QQmlPropertyCache::ConstPtr, 4> findPropertyCachesInCompositeTypes(
+            QMetaType t) const;
+
     static QQmlPropertyCache::ConstPtr propertyCacheForPotentialInlineComponentType(
             QMetaType t, const QQmlMetaTypeData::CompositeTypes::const_iterator &iter);
 

diff --git a/src/qml/qml/qqmlpropertyvalidator.cpp b/src/qml/qml/qqmlpropertyvalidator.cpp
index c252f47b73141e19923b1aa1e0d63cbd365ed260..1acbb2218508b2578ebf88a877a4e125897025ff 100644 (file)
--- a/src/qml/qml/qqmlpropertyvalidator.cpp
+++ b/src/qml/qml/qqmlpropertyvalidator.cpp
@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
 // Qt-Security score:significant
 
+#include "qqmlmetatype_p.h"
 #include "qqmlpropertyvalidator_p.h"
 
 #include <private/qqmlcustomparser_p.h>
@@ -781,10 +782,29 @@ QQmlError QQmlPropertyValidator::validateObjectBinding(const QQmlPropertyData *p
             // Will be true if the assigned type inherits propertyMetaObject
             // Determine isAssignable value
             bool isAssignable = false;
-            QQmlPropertyCache::ConstPtr c = propertyCaches.at(binding->value.objectIndex);
-            while (c && !isAssignable) {
-                isAssignable |= c == propertyMetaObject;
-                c = c->parent();
+            QQmlPropertyCache::ConstPtr source = propertyCaches.at(binding->value.objectIndex);
+
+            const auto inheritsFrom = [&](const QQmlPropertyCache::ConstPtr &target) {
+                for (QQmlPropertyCache::ConstPtr c = source; c; c = c->parent()) {
+                    if (c == target)
+                        return true;
+                }
+                return false;
+            };
+
+            isAssignable = inheritsFrom(propertyMetaObject);
+
+            if (!isAssignable) {
+                // A single (composite) metatype can map to multiple property caches when
+                // there are multiple engines; rawPropertyCacheForType only returns one of
+                // them. For non-composite types this yields an empty list and is a no-op.
+                const auto candidates = QQmlMetaType::rawCompositePropertyCachesForType(propType);
+                for (const auto &candidate : candidates) {
+                    if (inheritsFrom(candidate)) {
+                        isAssignable = true;
+                        break;
+                    }
+                }
             }
 
             if (!isAssignable) {

diff --git a/tests/auto/qml/qqmllanguage/data/canQmlCastMultiEngine/Foo.qml b/tests/auto/qml/qqmllanguage/data/canQmlCastMultiEngine/Foo.qml
new file mode 100644 (file)
index 0000000..8fc36a4
--- /dev/null
+++ b/tests/auto/qml/qqmllanguage/data/canQmlCastMultiEngine/Foo.qml
@@ -0,0 +1,3 @@
+import QtQml
+
+QtObject {}

diff --git a/tests/auto/qml/qqmllanguage/data/canQmlCastMultiEngine/Main.qml b/tests/auto/qml/qqmllanguage/data/canQmlCastMultiEngine/Main.qml
new file mode 100644 (file)
index 0000000..1470151
--- /dev/null
+++ b/tests/auto/qml/qqmllanguage/data/canQmlCastMultiEngine/Main.qml
@@ -0,0 +1,9 @@
+import QtQml
+
+QtObject {
+    property Foo theFoo: Foo {}
+
+    function check(foo: Foo) : bool {
+        return foo !== null;
+    }
+}

diff --git a/tests/auto/qml/qqmllanguage/data/propertyValidatorMultiEngine/Consumer1.qml b/tests/auto/qml/qqmllanguage/data/propertyValidatorMultiEngine/Consumer1.qml
new file mode 100644 (file)
index 0000000..80de08e
--- /dev/null
+++ b/tests/auto/qml/qqmllanguage/data/propertyValidatorMultiEngine/Consumer1.qml
@@ -0,0 +1,5 @@
+import QtQml
+
+QtObject {
+    property Foo f: Foo {}
+}

diff --git a/tests/auto/qml/qqmllanguage/data/propertyValidatorMultiEngine/Consumer2.qml b/tests/auto/qml/qqmllanguage/data/propertyValidatorMultiEngine/Consumer2.qml
new file mode 100644 (file)
index 0000000..80de08e
--- /dev/null
+++ b/tests/auto/qml/qqmllanguage/data/propertyValidatorMultiEngine/Consumer2.qml
@@ -0,0 +1,5 @@
+import QtQml
+
+QtObject {
+    property Foo f: Foo {}
+}

diff --git a/tests/auto/qml/qqmllanguage/data/propertyValidatorMultiEngine/Foo.qml b/tests/auto/qml/qqmllanguage/data/propertyValidatorMultiEngine/Foo.qml
new file mode 100644 (file)
index 0000000..8fc36a4
--- /dev/null
+++ b/tests/auto/qml/qqmllanguage/data/propertyValidatorMultiEngine/Foo.qml
@@ -0,0 +1,3 @@
+import QtQml
+
+QtObject {}

diff --git a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
index 4678949804e9eb5f646e4360e4c1af9c3c5226bd..8f28b87c8f299d2f4ad1d4e35f60223b68c5be11 100644 (file)
--- a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
+++ b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
@@ -298,6 +298,8 @@ private slots:
     void instanceof();
     void instanceofMultiEngine();
     void instanceofMultiEngineInlineComponent();
+    void propertyValidatorMultiEngine();
+    void canQmlCastMultiEngine();
 
     void concurrentLoadQmlDir();
 
@@ -6012,6 +6014,51 @@ void tst_qqmllanguage::instanceofMultiEngineInlineComponent()
     QVERIFY(result2.toBool());
 }
 
+// verify that the property validator does not get confused when
+// multiple engines end up registering the same type, leading to
+// multiple entries in compositeTypes
+void tst_qqmllanguage::propertyValidatorMultiEngine()
+{
+    QQmlEngine engine1;
+    QQmlComponent c1(&engine1, testFileUrl("propertyValidatorMultiEngine/Consumer1.qml"));
+    QVERIFY2(c1.isReady(), qPrintable(c1.errorString()));
+
+    QQmlEngine engine2;
+    QQmlComponent c2(&engine2, testFileUrl("propertyValidatorMultiEngine/Consumer1.qml"));
+    QVERIFY2(c2.isReady(), qPrintable(c2.errorString()));
+
+    QQmlComponent c3(&engine1, testFileUrl("propertyValidatorMultiEngine/Consumer2.qml"));
+    QVERIFY2(c3.isReady(), qPrintable(c3.errorString()));
+}
+
+// Verify that qmlobject_can_qml_cast() still recognizes a composite type when
+// multiple engines have loaded the same QML file
+void tst_qqmllanguage::canQmlCastMultiEngine()
+{
+    const QUrl url = testFileUrl("canQmlCastMultiEngine/Main.qml");
+
+    QQmlEngine engine1;
+    QQmlComponent c1(&engine1, url);
+    QVERIFY2(c1.isReady(), qPrintable(c1.errorString()));
+    QScopedPointer<QObject> obj1(c1.create());
+    QVERIFY(obj1);
+
+    // A second engine loads the same file and registers another compilation unit
+    // (and thus another property cache) for Foo.
+    QQmlEngine engine2;
+    QQmlComponent c2(&engine2, url);
+    QVERIFY2(c2.isReady(), qPrintable(c2.errorString()));
+    QScopedPointer<QObject> obj2(c2.create());
+    QVERIFY(obj2);
+
+    // Call the typed function on engine1's object with engine1's Foo instance.
+    QQmlExpression expr(engine1.contextForObject(obj1.data()), obj1.data(),
+                        QStringLiteral("check(theFoo)"));
+    const QVariant result = expr.evaluate();
+    QVERIFY2(!expr.hasError(), qPrintable(expr.error().description()));
+    QVERIFY(result.toBool());
+}
+
 void tst_qqmllanguage::concurrentLoadQmlDir()
 {
     ThreadedTestHTTPServer server(dataDirectory());